Skip to content

[Fix][Workflow] doxygen Doxyfile 返回错误 但是仍然生成成功 #10596

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

[Fix][Workflow] doxygen Doxyfile 返回错误 但是仍然生成成功 #10596

wants to merge 1 commit into from
+15 −6

Conversation

meng-plus
Copy link
Contributor

@meng-plus meng-plus commented Aug 9, 2025 •
edited
Loading

拉取/合并请求描述：(PR description)

[

为什么提交这份PR (why to submit this PR)

doxygen工作流总是出现错误 无法获得最新的官方文档

你的解决方案是什么 (what is your solution)

经过测试排查日志doxygen已经成功执行，但是不清楚为嘛给出错误结果导致终止，
其输出的html文档也是正常可使用的，因此做个条件拦截，即使提示失败也正常执行下去，

cat doxygen作用不明 应该无实际意义，因此移除

请提供验证的bsp和config (provide the config and bsp)

  • BSP:
  • .config:
  • action:

]

当前拉取/合并请求的状态 Intent for your PR

必须选择一项 Choose one (Mandatory):

  • 本拉取/合并请求是一个草稿版本 This PR is for a code-review and is intended to get feedback
  • 本拉取/合并请求是一个成熟版本 This PR is mature, and ready to be integrated into the repo

代码质量 Code Quality：

我在这个拉取/合并请求中已经考虑了 As part of this pull request, I've considered the following:

  • 已经仔细查看过代码改动的对比 Already check the difference between PR and old code
  • 代码风格正确，包括缩进空格，命名及其他风格 Style guide is adhered to, including spacing, naming and other styles
  • 没有垃圾代码，代码尽量精简，不包含#if 0代码，不包含已经被注释了的代码 All redundant code is removed and cleaned up
  • 所有变更均有原因及合理的，并且不会影响到其他软件组件代码或BSP All modifications are justified and not affect other components or BSP
  • 对难懂代码均提供对应的注释 I've commented appropriately where code is tricky
  • 代码是高质量的 Code in this PR is of high quality
  • 已经使用formatting 等源码格式化工具确保格式符合RT-Thread代码规范 This PR complies with RT-Thread code specification
  • 如果是新增bsp, 已经添加ci检查到.github/workflows/bsp_buildings.yml 详细请参考链接BSP自查

@Copilot Copilot AI review requested due to automatic review settings August 9, 2025 18:41
@meng-plus meng-plus requested a review from supperthomas as a code owner August 9, 2025 18:41
@github-actions github-actions bot added the action github action yml imporve label Aug 9, 2025
@github-actions GitHub Actions
Copy link

github-actions bot commented Aug 9, 2025 •
edited
Loading

📌 Code Review Assignment

🏷️ Tag: workflow

Reviewers: Rbb666 kurisaW supperthomas

Changed Files (Click to expand)
  • .github/workflows/doxygen.yml

📊 Current Review Status (Last Updated: 2025-08-11 16:10 CST)

  • ⌛ Rbb666 Pending Review
  • ⌛ kurisaW Pending Review
  • ⌛ supperthomas Pending Review

📝 Review Instructions

  1. 维护者可以通过单击此处来刷新审查状态: 🔄 刷新状态
    Maintainers can refresh the review status by clicking here: 🔄 Refresh Status

  2. 确认审核通过后评论 LGTM/lgtm
    Comment LGTM/lgtm after confirming approval

  3. PR合并前需至少一位维护者确认
    PR must be confirmed by at least one maintainer before merging

ℹ️ 刷新CI状态操作需要具备仓库写入权限。
ℹ️ Refresh CI status operation requires repository Write permission.

Copilot
Copilot AI reviewed Aug 9, 2025
View reviewed changes
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR fixes an issue where the Doxygen workflow was failing due to errors in the Doxyfile configuration, but still needed to generate documentation successfully. The change modifies error handling to allow the workflow to continue even when Doxygen encounters errors.

Key changes:

  • Wrapped the doxygen Doxyfile command in conditional error handling
  • Added verification to check if HTML documentation was generated successfully
  • Removed the cat Doxyfile command which appears to be debug code

.github/workflows/doxygen.yml
echo "Doxygen failed, but workflow will continue."
fi
# 检查生成结果
Copy link
Preview

Copilot AI Aug 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] The comment is in Chinese while the rest of the workflow file appears to use English. Consider using English for consistency: '# Check generation results'.

Suggested change
# 检查生成结果
# Check generation results

Copilot uses AI. Check for mistakes.

.github/workflows/doxygen.yml
Comment on lines +58 to +59
if ! doxygen Doxyfile; then
echo "Doxygen failed, but workflow will continue."
Copy link
Preview

Copilot AI Aug 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider capturing and logging the specific Doxygen error output to aid in debugging. You could redirect stderr to a variable or file for later inspection.

Suggested change
if ! doxygen Doxyfile; then
echo "Doxygen failed, but workflow will continue."
if ! doxygen Doxyfile 2> doxygen_error.log; then
echo "Doxygen failed, but workflow will continue."
echo "---- Doxygen error output ----"
cat doxygen_error.log
echo "-----------------------------"

Copilot uses AI. Check for mistakes.

kurisaW
kurisaW reviewed Aug 11, 2025
View reviewed changes
@unicornx
Copy link
Contributor

为啥要提交这个 pr？PR 中能够详细描述一下？
我看了代码修改，猜测是不是作者觉得原来没有判断 doxygen 执行失败的情况，所以这次要加上判断。

但是比较奇怪的是，原来代码中执行完 doxygen 后会运行 cat，这个 pr 修改把 cat 这一步直接删掉了，这是何考虑？原来 cat 的目的是啥？这次修改有没有破坏原来的逻辑？

@meng-plus
Copy link
Contributor Author

为啥要提交这个 pr？PR 中能够详细描述一下？
我看了代码修改，猜测是不是作者觉得原来没有判断 doxygen 执行失败的情况，所以这次要加上判断。

但是比较奇怪的是，原来代码中执行完 doxygen 后会运行 cat，这个 pr 修改把 cat 这一步直接删掉了，这是何考虑？原来 cat 的目的是啥？这次修改有没有破坏原来的逻辑？

之前的并未执行到cat就报错退出了 所以每次的pr提交 都会提示这个流程报错

doxygen的执行结果返回值 或者其他问题 导致了 流程终止， 我在本地测试的测试下如此调整输出了结果是正常可用的

@unicornx
Copy link
Contributor

unicornx commented Aug 11, 2025 •
edited
Loading

为啥要提交这个 pr？PR 中能够详细描述一下？
我看了代码修改，猜测是不是作者觉得原来没有判断 doxygen 执行失败的情况，所以这次要加上判断。
但是比较奇怪的是，原来代码中执行完 doxygen 后会运行 cat，这个 pr 修改把 cat 这一步直接删掉了，这是何考虑？原来 cat 的目的是啥？这次修改有没有破坏原来的逻辑？

之前的并未执行到cat就报错退出了 所以每次的pr提交 都会提示这个流程报错

doxygen的执行结果返回值 或者其他问题 导致了 流程终止， 我在本地测试的测试下如此调整输出了结果是正常可用的

没看懂你的回复到底是回答了什么问题。我再把我的问题总结一下吧：

我的问题来源于我认为这个 PR 的改动主要体现为两部分：

第一部分：增加了对 doxygen Doxyfile 的结果进行判断，针对这个改动我的建议是请补充修改原因，RTT 提交 PR 的模板已经清楚地提出了要求，请补充完整方便大家 review， 至于代码改动我暂时没有问题。

image

第二部分：这个 PR 删除了原代码中的 cat Doxyfile 语句。这个改动我有疑问是不是改错了？原来的代码这么写是何原因？如果确认原来的写法是不需要的，那删掉就没有问题。

@supperthomas supperthomas added the -1 No vote label Aug 11, 2025
@supperthomas
Copy link
Member

doxygen有错误，需要修复错误，才能CI通过。

@supperthomas
Copy link
Member

cat Doxyfile
为了方便检查Doxyfile是否符合预期。和是否和本地一致

@unicornx
Copy link
Contributor

unicornx commented Aug 11, 2025 •
edited
Loading

doxygen有错误，需要修复错误，才能CI通过。

doxygen 的buld错误我修好了，见

但我觉得本 PR 增加对 doxygen build 失败的检查还是有必要的。而且我建议以后对所有 pr 有注释修改的情况都 run doxygen 的 ci 检查。如果觉得检查 pr 是否修改了注释比较麻烦，也可以每个 pr 都强制检查 doxygen build。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kurisaW kurisaW kurisaW left review comments

Copilot code review Copilot Copilot left review comments

@supperthomas supperthomas Awaiting requested review from supperthomas supperthomas is a code owner

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
-1 No vote action github action yml imporve
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@meng-plus @unicornx @supperthomas @kurisaW